Skip to content

hmon: use single Arc for heartbeat monitor#297

Merged
pawelrutkaq merged 3 commits into
eclipse-score:mainfrom
qorix-group:arkjedrz_heartbeat-monitor-use-single-arc
Jul 13, 2026
Merged

hmon: use single Arc for heartbeat monitor#297
pawelrutkaq merged 3 commits into
eclipse-score:mainfrom
qorix-group:arkjedrz_heartbeat-monitor-use-single-arc

Conversation

@arkjedrz

@arkjedrz arkjedrz commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Replace previous approach with two Arcs.

@arkjedrz arkjedrz requested a review from pawelrutkaq July 8, 2026 13:53
@arkjedrz arkjedrz self-assigned this Jul 8, 2026
@arkjedrz arkjedrz requested a deployment to workflow-approval July 8, 2026 13:54 — with GitHub Actions Waiting
@arkjedrz arkjedrz requested a deployment to workflow-approval July 8, 2026 13:54 — with GitHub Actions Waiting
@github-actions

github-actions Bot commented Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output 
[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.6.0) and connecting to it...
INFO: Invocation ID: c1cf7a72-3964-43a6-acea-fb4fb5b9d870
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (37 packages loaded, 10 targets configured)

Analyzing: target //:license-check (84 packages loaded, 10 targets configured)

Analyzing: target //:license-check (86 packages loaded, 10 targets configured)

Analyzing: target //:license-check (86 packages loaded, 10 targets configured)

Analyzing: target //:license-check (141 packages loaded, 2448 targets configured)

Analyzing: target //:license-check (152 packages loaded, 5481 targets configured)

Analyzing: target //:license-check (153 packages loaded, 7239 targets configured)

Analyzing: target //:license-check (157 packages loaded, 8219 targets configured)

Analyzing: target //:license-check (164 packages loaded, 8271 targets configured)

Analyzing: target //:license-check (164 packages loaded, 8271 targets configured)

Analyzing: target //:license-check (167 packages loaded, 10159 targets configured)

Analyzing: target //:license-check (167 packages loaded, 10159 targets configured)

Analyzing: target //:license-check (168 packages loaded, 10283 targets configured)

INFO: Analyzed target //:license-check (169 packages loaded, 10409 targets configured).
[14 / 16] [Prepa] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar
[15 / 16] Building license.check.license_check.jar (); 0s disk-cache, multiplex-worker
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 25.943s, Critical Path: 2.55s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

The created documentation from the pull request is available at: docu-html

@arkjedrz arkjedrz force-pushed the arkjedrz_heartbeat-monitor-use-single-arc branch from 0e2595b to bec63a8 Compare July 9, 2026 06:38
@arkjedrz arkjedrz requested a deployment to workflow-approval July 9, 2026 06:38 — with GitHub Actions Waiting
@arkjedrz arkjedrz requested a deployment to workflow-approval July 9, 2026 06:38 — with GitHub Actions Waiting
danth
danth reviewed Jul 9, 2026
View reviewed changes

@danth danth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, calling get_eval_handle a second time for the same monitor would return a new handle with a separate timestamp. After this change, the same timestamp is shared between all handles.

I'm not sure what the expected behaviour should be, but it would be good to add a test for this either way.

@arkjedrz arkjedrz force-pushed the arkjedrz_heartbeat-monitor-use-single-arc branch from bec63a8 to 3d076e5 Compare July 10, 2026 11:26
@arkjedrz arkjedrz requested a deployment to workflow-approval July 10, 2026 11:26 — with GitHub Actions Waiting
@arkjedrz arkjedrz requested a deployment to workflow-approval July 10, 2026 11:26 — with GitHub Actions Waiting
@arkjedrz arkjedrz requested a deployment to workflow-approval July 10, 2026 11:53 — with GitHub Actions Waiting
@arkjedrz arkjedrz requested a deployment to workflow-approval July 10, 2026 11:53 — with GitHub Actions Waiting
@arkjedrz

arkjedrz commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@danth, thanks for taking a look.
Each monitor should have get_eval_handle used once - by HealthMonitor::get_monitor, so the issue would never be observed using public APIs. This method is returning handles to the same monitor, so it was not a right choice to have a stateful handle in the first place. I added a test case for that, it is not passing for main branch.
BTW the context of this change is to enable protected memory allocator, having a double Arc unnecessarily complicates things.

@arkjedrz arkjedrz force-pushed the arkjedrz_heartbeat-monitor-use-single-arc branch from a19e15f to c1c7f91 Compare July 10, 2026 14:01
@arkjedrz arkjedrz temporarily deployed to workflow-approval July 10, 2026 14:01 — with GitHub Actions Inactive
@arkjedrz arkjedrz temporarily deployed to workflow-approval July 10, 2026 14:01 — with GitHub Actions Inactive
@arkjedrz arkjedrz force-pushed the arkjedrz_heartbeat-monitor-use-single-arc branch from c1c7f91 to cb4772c Compare July 10, 2026 14:45
@arkjedrz arkjedrz temporarily deployed to workflow-approval July 10, 2026 14:45 — with GitHub Actions Inactive
@arkjedrz arkjedrz temporarily deployed to workflow-approval July 10, 2026 14:45 — with GitHub Actions Inactive
@pawelrutkaq pawelrutkaq merged commit 7993e19 into eclipse-score:main Jul 13, 2026
24 of 25 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in LCM - Lifecycle & Health FT Jul 13, 2026
@arkjedrz arkjedrz deleted the arkjedrz_heartbeat-monitor-use-single-arc branch July 13, 2026 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pawelrutkaq pawelrutkaq pawelrutkaq approved these changes
+1 more reviewer
@danth danth danth left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@arkjedrz arkjedrz

Labels

None yet

Projects

LCM - Lifecycle & Health FT
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@arkjedrz @danth @pawelrutkaq